Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vendor: bump Pebble to 59007c613a66 #71405

Merged
merged 1 commit into from
Oct 12, 2021

Conversation

nicktrav
Copy link
Collaborator

59007c61 sstable: consolidate checksum types
17635b0a *: address staticcheck lints
11823273 *: address staticcheck lint check U1000
807abfe8 *: address staticcheck lint check S1039
d24dd342 all: remove some unused code
3bdca93a sstable: clarify comment on checksum input
926d23e9 pebble: remove comment related to batching from the table cache
0a6177ae db: tweak ErrClosed documentation
ecc685b2 db: expose facility for retrieving batch count
b2eb88a7 sstable: remove unused rawBlockIter err field

Release note: None.

@nicktrav nicktrav requested a review from a team October 11, 2021 16:37
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Collaborator

@jbowens jbowens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ecc685b2 db: expose facility for retrieving batch count

Heads up that I think this commit will require a code change in some (one?) places in pkg/storage. pebble.NewBatchReader now returns another return value.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

Copy link
Collaborator Author

@nicktrav nicktrav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yep. Just caught it. Thanks!

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

```
59007c61 sstable: consolidate checksum types
17635b0a *: address staticcheck lints
11823273 *: address staticcheck lint check U1000
807abfe8 *: address staticcheck lint check S1039
d24dd342 all: remove some unused code
3bdca93a sstable: clarify comment on checksum input
926d23e9 pebble: remove comment related to batching from the table cache
0a6177ae db: tweak ErrClosed documentation
ecc685b2 db: expose facility for retrieving batch count
b2eb88a7 sstable: remove unused rawBlockIter err field
```

Adds a explicit size-check to error on uint -> int overflow.

Release note: None.
@nicktrav nicktrav force-pushed the nickt/pebble-master-59007c613a66 branch from 6729eaf to 8bb62b2 Compare October 11, 2021 17:04
Copy link
Collaborator Author

@nicktrav nicktrav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained


pkg/storage/batch.go, line 127 at r2 (raw file):

	}
	r, c := pebble.ReadBatch(repr)
	if c > math.MaxInt32 {

@jbowens - looks like we were just casting before, so I added a check. Not sure if it's possible in practice to overflow, and can remove if obsolete.

Copy link
Collaborator

@jbowens jbowens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewable status: :shipit: complete! 1 of 0 LGTMs obtained

@nicktrav
Copy link
Collaborator Author

bors r=jbowens

@craig
Copy link
Contributor

craig bot commented Oct 12, 2021

Build succeeded:

@craig craig bot merged commit 6159a73 into cockroachdb:master Oct 12, 2021
@nicktrav
Copy link
Collaborator Author

TFTR!

@nicktrav nicktrav deleted the nickt/pebble-master-59007c613a66 branch October 12, 2021 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants